Tracking mode #105
Tracking mode #105
Conversation
wrote sort function can be made into database not entirely sure the build database stage is correct though (in io.cpp)
binary search algorithm
for convenience
vote by rotation of star pairs instead
added warning when forgetting to pass in prev attitude fixed the way prevAttitude is being passed in (as member of the star id algo class instead of part of the pipeline)
is already fixed in master but didn't realize it's not fixed on this branch!
I copy/pasted stuff from left loop to right and missed a few things to change Also fixed conditionals for loop Also made sure to assert radius is nonnegative, added to man page
use conjugates and angles to determine equality
no reason to use radius^2 had previously thought the < was wrong in the else if so I changed it to > but no, < is correct
also normalize CameraToSpatial outputs and a debug thing for query
1's and l's look too similar 😭
I was doing a binary search through the catalog instead of the sorted indices list also has a temp fix cuz sometimes the binary search comes up blank when it should have something
assuming rotations should preserve distance between stars
(where the query returns 1 star) also more debugging code to take out later
already fixed in master but that was before I made this branch
I should've just been considering if x is within radius for the starting index since that's what it's sorted by
…o tracking_mode
|
Instead of having a separate tracking mode database, let's just always sort the catalog by x-value in CatalogRead. There are a number of other situations where sorting based on x-value is useful (in pyramid), and it shouldn't break anything. |
| public: | ||
| TrackingSortedDatabase(const unsigned char *databaseBytes); | ||
| const static int32_t kMagicValue = 0x2536f0A9; | ||
| std::vector<int16_t> QueryNearestStars(const Catalog &, const Vec3 point, float radius); |
There was a problem hiding this comment.
Should have short documentation comment in header file
|
|
||
| .TP | ||
| \fB--uncertainty\fP \fIradius\fP | ||
| For tracking mode. \fIradius\fP gives the boundary for how much the field of view could have rotated. Defaults to 0.001 if not selected. Must be nonnegative. |
There was a problem hiding this comment.
What are the units? Degrees, pixels?
Also, the name is too short, the cli option name should make it clear that it's talking about uncertainty in the previous attitude.
|
|
||
| .TP | ||
| \fB--tracking-compare-threshold\fP \fIthreshold\fP | ||
| \fIthreshold\fP is the threshold for comparing floats for tracking mode. Defaults to 0.001 if no value is given. |
There was a problem hiding this comment.
What are the units? What exactly is being compared?
| float radius = 0.001; | ||
| float threshold = 0.001; | ||
|
|
||
| std::vector<int16_t> query_ind = db.QueryNearestStars(catalog, point, radius+threshold); |
There was a problem hiding this comment.
naming convention, no underscores
There was a problem hiding this comment.
There's a lot of duplication between the different uncertainty cases in this file. Instead of putting the actual testing code in SECTIONS, put the testing code after the SECTION, and use the SECTION only to set the variables, eg:
float radius, threshold;
SECTION("small uncertainty") {
radius = X;
threshold = Y;
}
SECTION("medium uncertainty") {
// ...
}
// actual testing code here
It's also possible to achieve a similar effect using the GENERATE macro.
|
|
||
| // convert user inputted string ra,dec,roll to individual floats | ||
| std::vector<float> attInputs; | ||
| std::stringstream ss (values.prevAttitudeString); |
There was a problem hiding this comment.
Taking attitude input as a single comma-separated string is a good idea, but we need to apply it consistently across the codebase. If we're going to do it here, we also need to replace the --generate-ra, --generate-de, and --generate-roll options with a single combined option (and there should be a separate function for parsing comma-separated attitudes, in attitude-utils.cpp)
| @@ -503,4 +504,37 @@ StarIdentifiers PyramidStarIdAlgorithm::Go( | |||
| return identified; | |||
| } | |||
|
|
|||
| // for ordering Quaternions in tracking mode votes map | |||
| bool operator<(const Quaternion& l, const Quaternion& r) { | |||
There was a problem hiding this comment.
This is not an appropriate general-purpose comparison for quaternions, just write it as a separate function and pass as a custom comparator to std::sort
Also, || is your friend.
| public: | ||
| PrevAttitude(Attitude prev, float uncertainty, float compareThreshold) | ||
| : prev(prev), uncertainty(uncertainty), compareThreshold(compareThreshold) { }; | ||
| PrevAttitude() |
There was a problem hiding this comment.
Do you need this no-argument constructor?
| @@ -60,6 +60,19 @@ const CatalogStar *findNamedStar(const Catalog &, int name); | |||
| // (so 523 = 5.23) | |||
| Catalog NarrowCatalog(const Catalog &, int maxMagnitude, int maxStars); | |||
|
|
|||
| // for tracking mode | |||
There was a problem hiding this comment.
Needs documentation comments for doxygen
A working tracking mode (#87). Can still use more optimization for if not enough stars are found for the attitude estimation algorithms.